Validate MessagePack enum values#38204
Closed
smoogipoo wants to merge 2 commits into
Closed
Conversation
smoogipoo
added
the
area:online functionality
smoogipoo
commented
Jul 1, 2026
Comment on lines
-55
to
+64
| baseFormatter = StandardResolver.Instance.GetFormatter<TBase>(); | ||
| baseFormatter = StandardResolver.Instance.GetFormatterWithVerify<TBase>(); |
Contributor
Author
There was a problem hiding this comment.
The "WithVerify" part of this checks that the formatter is not null. Was just looking through the MsgPack source and this is kind of how they do it, plus I needed the non-nullability.
bdach
requested changes
Jul 1, 2026
Comment on lines
+108
to
+109
| if (Enum.IsDefined(typeof(T), value)) | ||
| return; |
Collaborator
There was a problem hiding this comment.
[Flags] enums need to be exempted from this because there not being "defined" is actually a feature if you have a union of 2 or more flags.
This is actually relevant because we have one of those that goes over the wire to spectator server and it's ReplayButtonState.
diff --git a/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs b/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
index 6891b214d0..08e3e3a660 100644
--- a/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
+++ b/osu.Game.Tests/Online/TestMultiplayerMessagePackSerialization.cs
@@ -7,6 +7,7 @@
using osu.Game.Online;
using osu.Game.Online.Multiplayer;
using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus;
+using osu.Game.Replays.Legacy;
namespace osu.Game.Tests.Online
{
@@ -86,6 +87,9 @@ public void TestEnumValidation()
MessagePackSerializer.Deserialize<SimpleEnum?>(MessagePackSerializer.Serialize((SimpleEnum?)null, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
Assert.Throws<MessagePackSerializationException>(() =>
MessagePackSerializer.Deserialize<SimpleEnum?>(MessagePackSerializer.Serialize((SimpleEnum)100, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
+
+ Assert.DoesNotThrow(() =>
+ MessagePackSerializer.Deserialize<ReplayButtonState>(MessagePackSerializer.Serialize(ReplayButtonState.Left1 | ReplayButtonState.Right2, SignalRUnionWorkaroundResolver.OPTIONS), SignalRUnionWorkaroundResolver.OPTIONS));
}
public enum SimpleEnum
The above fails when run.
Contributor
Author
There was a problem hiding this comment.
I'm just gonna go ahead and close this PR for now... Detecting flags enums would require reflection, and at which point I'd rather be looking towards other more full-fledged validation mechanisms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.